-
Notifications
You must be signed in to change notification settings - Fork 241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Sana .pth files into download counting in huggingface repo #1061
Conversation
This reverts commit 4a78d08.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @lawrence-cj left 2 quick suggestions!
Co-authored-by: vb <[email protected]>
Cool! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there, thanks for the PR and the back and forth with @Vaibhavs10 :) I left 2 comments and then we should be good to go.
prettyLabel: "Sana", | ||
repoName: "Sana", | ||
repoUrl: "https://github.com/NVlabs/Sana", | ||
countDownloads: `path_extension:"pth" OR path_extension:"json"`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.pth file each time the pipeline is instantiated. Is that the case or not? And is the config.json ever loaded from script?
- If the script only loads the .pth file, then let's count only on .pth:
countDownloads: `path_extension:"pth" OR path_extension:"json"`, | |
countDownloads: `path_extension:"pth"`, |
- If the script loads the config file as well, then it's even better and we should count these calls and only these calls. config.json is already tracked by default and that's why some count are already listed on the Hub (see https://huggingface.co/Efficient-Large-Model/Sana_1600M_512px). In that case, no need to add a countDownloads rule.
countDownloads: `path_extension:"pth" OR path_extension:"json"`, |
I let you decide what's best for Sana library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, we didn't have a config.json file until yesterday. We want to recover it, so we add pth here. Is this reasonable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, makes sense to track pth files then 👍
Co-authored-by: Lucain <[email protected]>
Co-authored-by: Lucain <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Lint is finally passing 😄 |
OHH, Great to know. Thanks a lot for your support and quick apply @Wauplin @Vaibhavs10 |
Hi experts. Sorry for interrupting. The downloading count does not recover in our collection. For example, https://huggingface.co/Efficient-Large-Model/Sana_1600M_1024px |
Hi @lawrence-cj , sorry about the delay. The release did not make it to production yet. I've opened an internal PR to make it happen. |
Hey @lawrence-cj I can confirm to you that this PR has been deployed to production. I can now see 7400+ downloads for https://huggingface.co/Efficient-Large-Model/Sana_1600M_1024px for instance. Is that more in line with what you were expecting? |
Oh, thank you so much for it @Wauplin. Actually, I would have expected more as we have 70K downloads of our component models. But that's ok, I'm ok if that's the backend data. |
No description provided.